Conversation
| std::array<char, 128> close_to_open{}; | ||
| close_to_open[')'] = '('; | ||
| close_to_open['}'] = '{'; | ||
| close_to_open[']'] = '['; |
There was a problem hiding this comment.
map ではなく array にしたのは意図的でしょうか。小さいものなら単純な array の方が効率がよい、というのはあるかもしれませんね。
There was a problem hiding this comment.
@ryosuketc
ご返信が随分と遅くなり大変申し訳ございません。
こちらを書いた時は、「弧の種類(3種)が少なく、拡張を考えなくていいから、速さとシンプルさを優先して array を選んだ」形ですね。
新たに括弧を追加する場面も考えると、unordered_mapで書いた方がいいかもしれないですね。
| ->開き括弧を確認後、対応する閉じ括弧そのものをスタックに積む。閉じ括弧が来たらスタックの先頭と一致するかだけを見るので、マップも比較分岐も最小限で済ませる。 | ||
|
|
||
| - [配列テーブル(closing→opening)+文字列をスタックとして使う] (別解2) | ||
| ->unordered_map の代わりに 固定長配列(ASCII 128) を使って 閉じ括弧→対応する開き括弧 を O(1) 参照。スタックは std::string を使うと軽量(push/pop_back)。 |
There was a problem hiding this comment.
スタックは std::string を使うと軽量(push/pop_back)。
この発想はありませんでした。ただ std::stack と比べて可読性が下がるように感じます。。
| return false; | ||
| } | ||
|
|
||
| std::array<char, 128> close_to_open{}; |
There was a problem hiding this comment.
{} を付けると 0 で初期化されるのだと思いますが、以下で必要な要素に代入しているため、 {} を外して初期化なしにしてもよいと思いました。
There was a problem hiding this comment.
@nodchip
ご返信が随分と遅くなり大変申し訳ございません。
ご指摘頂きありがとうございます。
ここでは、'(', '{', '['の3つだけなので、後々代入するのであれば、初期化なしにした方がいいかもしれないですね。
色々と調べた時に、未代入のインデックスにアクセスすると 未定義動作(UB)になる可能性がある(https://en.cppreference.com/w/cpp/language/ub.html)と記載があったので、初期化をしてました。
順に書いていった時に、必要・不必要なこと当たり前に書けるようにします。
| public: | ||
| bool isValid(string& s) { | ||
| unordered_map<char, char> open_to_close = { | ||
| {'(', ')'}, |
| std::stack<char> st; | ||
|
|
||
| for (char c : s) { | ||
| switch (c) { |
There was a problem hiding this comment.
好みの範囲かもしれませんが、括弧の種類が増えたときの拡張性なども考えると括弧はmap等で管理し、ifで分岐処理を記述したほうが簡潔なように思います。
There was a problem hiding this comment.
@maeken4
ご返信随分と遅くなり大変申し訳ありません。
ご指摘ありがとうございます。他のパターンを意識した記載ができておりませんでした。
問題文の範囲外でも一般的に考えられる所まで気をつけたいと思います。
This: https://leetcode.com/problems/valid-parentheses/
Next: https://leetcode.com/problems/reverse-linked-list/description/